feat(admin): scoped user-management with approval queue (#64, #85)#91
Open
martinydeAI wants to merge 4 commits into
Open
feat(admin): scoped user-management with approval queue (#64, #85)#91martinydeAI wants to merge 4 commits into
martinydeAI wants to merge 4 commits into
Conversation
Lands the User-entity work of ADR 006 — a `UserStatus` PHP enum (`pending | approved | blocked`) and a matching `status` column on the entity, plus the long-pending `name` display column. The `UserManager::createUser()` service grows a required `name` argument and an optional `status` (defaults to `Approved` for the console / fixture path; the registration flow in #62 will pass `Pending`). The `app:user:create` console command grows a third `name` argument; fixtures seed Alice + Bob with display names. A single migration adds both columns with a default value so existing rows in long-running environments backfill cleanly: `name = ''` and `status = 'approved'` (historic rows are assumed already-trusted accounts). Combines #45 (User.name) and #83 (UserStatus enum + status field) in one PR, since the migration is most coherent as a single ALTER TABLE and the entity tests fight if either field lands without the other. Sequenced as PR 1 of the User management milestone plan (`docs/plans/user-management-milestone.md`, written locally) — PR 3 (UserChecker, #63) and PR 4 (Registration, #62) stack on top of this branch with `do-not-merge` labels until it merges. Closes #45. Closes #83. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
# Conflicts: # CHANGELOG.md
Adds the admin user-management surface decided in ADR 006:
- `App\Controller\Admin\UserController` exposes `/admin/users` (list),
`/admin/users/pending` (redirects to the pending-filtered list), and
POST-only `/admin/users/{id}/{approve|block}` actions. Class-level
`IsGranted('ROLE_DOMAIN_MANAGER')` plus per-action
`IsGranted('APPROVE_USER'|'BLOCK_USER', subject: $user)` gates
through PR 2's `ManageUserVoter` so the same-domain check fires
on every state transition.
- `App\Security\UserApproval` owns the `Pending -> Approved` and
`Approved -> Blocked` transitions (and their reverses). One place
to add audit logging later if needed.
- `App\Repository\UserRepository::findVisibleTo()` scopes the list
query: admins see every user, domain managers see only same-domain
users via `LOWER(email) LIKE '%@<domain>'`, anyone else sees an
empty result. Optional status filter via the `UserStatus` enum so
`/admin/users?status=pending` powers the approval-queue subset.
- `templates/admin/user/list.html.twig` renders the table with the
existing `Form/Button` + `Eyebrow` components and per-row action
forms. CSRF-protected via Symfony's `csrf_token('admin-user-action')`
helper; the `back` parameter on each action only honours `/admin/users…`
URLs so a hostile actor can't redirect off-site.
- `security.yaml` adds an `^/admin` access-control rule as a second
layer in front of the controller `IsGranted`.
Sequenced as PR 5 of the User management milestone plan. Branched
from PR 2 (#87, voter + Roles + EmailDomain) and merged in PR 1
(#86, User.name + User.status) since both are needed; labelled
`do-not-merge` until both bases land. Closes #12's broader RBAC
umbrella along with the concrete sibling closure plan.
Closes #64.
Closes #85.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Removed references to ADR 006 and added clarity to column descriptions.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Links to issues
Closes #64.
Closes #85.
Implements PR 5 of the User management milestone plan.
! This pr should not be handled before other User management PRs have been merged to keep a proper comparison.
Description
Admin user-management surface:
GET /admin/users— list view scoped by role. Admins see everyuser, domain managers see only same-domain users (via the
EmailDomainhelper). Optional?status=pending|approved|blockedfilter powers the approval queue (feat: approval queue for domain managers #64).
GET /admin/users/pending— sugar route, 302 to/admin/users?status=pendingso feat: approval queue for domain managers #64's URL keeps working.POST /admin/users/{id}/approveand.../block— flip the user'sstatus. Gated by
IsGranted('APPROVE_USER' / 'BLOCK_USER', subject: $user)so PR feat(security): ROLE_DOMAIN_MANAGER, role hierarchy, domain-scoped voter (#84) #87's voter fires on every transition. CSRF-protectedwith the
admin-user-actionintent token.App\Security\UserApproval— single home for thePending -> Approved/Approved -> Blockedtransitions.App\Repository\UserRepository::findVisibleTo()— scoped finderwith optional status filter. Domain managers without an email
defensively get an empty result (defence in depth — the firewall
rather than running a query against an unresolved domain).
^/adminaccess_controlrule + per-actionIsGranted—belt-and-braces.
Form/Button,Form/Label,Form/TextInput,Eyebrow, and base layout — no new component families.backparameter on the action forms only honours/admin/users…URLs (hostile-redirect guard).
Screenshot of the result
n/a — UI matches the existing layout primitives. Manual screenshot
can be added on request.
Checklist
Verified locally:
task coding-standards-check— all green.task test-coverage— 85 tests, 252 assertions, 100 %coverage.
Additional comments or questions
The "domain manager promotion" UI is not in scope. Granting a
user
ROLE_DOMAIN_MANAGER(or revoking it) is still done via theconsole for now — separate UX decision, not tracked as its own issue
yet.
The voter's
IsGrantedruns before the controller body, so across-domain POST yields 403 from the voter — never reaching the
CSRF check. Both paths are exercised by the test suite.
#12(broader RBAC meta) can be closed when this PR lands per theplan — its concrete payload is now the role set in #84 + the scoped
admin access here.
Details - AI specificities
docs/adr/006-user-approval-and-account-state.md(Draft; lands via docs: add ADR 004 — user registration, approval, and account state #61). Specifically the "Approval queue" and
"Domain manager — a role, not a flag" sections.
develop.do-not-mergelabel.and feat(security): ROLE_DOMAIN_MANAGER, role hierarchy, domain-scoped voter (#84) #87.
UserRepositoryTestgets the threenew methods (
testFindVisibleToReturnsEveryUserForAdmin,testFindVisibleToScopesByDomainForDomainManager,testFindVisibleToFiltersByStatus) the user approved earlier. Thescoping test absorbs the "non-manager empty" and
"domain-manager-without-email empty" branches as sub-assertions
rather than adding a 4th method, to stay within the approved
boundary.
grows.
ROLE_DOMAIN_MANAGER— separate concern.admin-user-action. Same token works for bothapprove + block actions; the route + voter combination is what
distinguishes the actual operation.